-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
crypto: fix base58 encoding for BlsSignature #56
Conversation
6581ee5
to
f625864
Compare
This is true, but 128 bytes is not the same as 128 base58 characters.
This is false, So this was never a problem with dependencies. I thought I explained it in the issue. |
f625864
to
1e90557
Compare
Ah thanks, apologies for the misunderstanding. Have updated |
Hmm. I did some testing, and apparently I was under a few misconceptions. Apologies. So while this works fine for bona fide Tezos hashes, there's an unfortunate landmine in the Hence, it turns out, the library switch was a better overall fix. Can you revert back to f625864? Again, sorry for this. |
1e90557
to
4e39b21
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a couple suggestions.
The current base58 dep has a 128 byte limit, which is not long enough for bls sigs
4e39b21
to
350f51e
Compare
The current base58 module incorrectly had a
128
char limit set on decoding hashes - too small forbls sigs
. We raise this limit to the length ofBlsSigs
hashes, which are the longest hashes we support.fixes #49